Skip to content

Hardcode get_tolerance for Float64 for trimming #648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 27, 2025

Conversation

RomeoV
Copy link
Contributor

@RomeoV RomeoV commented Jul 26, 2025

This is definitely a bug somewhere, but without this @report_opt or @report_trim reports errors here.

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

Add any other context about the problem here.

This is definitely a bug somewhere, but without this `@report_opt` or
`@report_trim` reports errors here.
Copy link
Member

@oscardssmith oscardssmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can probably do better here. this works, but there's really no reason we should be doing a rational number power for a heuristic like this.

@ChrisRackauckas
Copy link
Member

Can you give a concrete suggestion? I think hardcoding Float64 and Float32 is reasonable.

@oscardssmith
Copy link
Member

what is this number actually used for? Default nonlinear solve tolerance?

@ChrisRackauckas
Copy link
Member

Yes. And JET really dislikes it. So, let's do it and then if you have concrete improvements let's follow up, but it's strictly better to just add this for now.

@oscardssmith
Copy link
Member

oscardssmith commented Jul 27, 2025

right, but is there a reason we aren't just using 100*eps()? What's special about x^0.8? (or we could just use sqrt(sqrt(x^3)) if we care about it being a power for some reason)

@ChrisRackauckas
Copy link
Member

I think @avik-pal had a reason for that.

@ChrisRackauckas ChrisRackauckas merged commit e9e4166 into SciML:master Jul 27, 2025
76 of 92 checks passed
@RomeoV RomeoV deleted the common-defaults-trim branch August 1, 2025 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants